Repository navigation
Conversation
Delta time travel picks a version by each commit log's modification time, not by the timestamp recorded inside the log. The fixture wrote two commits back to back, a millisecond or two apart, and pinned at the first commit's recorded timestamp, so any coarsening of the mtime clock resolved the pin to the in-flight overwrite instead of the completed snapshot. Stamp both commit logs explicitly and pin between them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
❌ This pull request could not start testing because there was a merge conflict. See more details here.
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
🤖 CI report
|
|
Reviews (1): Last reviewed commit: "fix(warehouse-sources): pin the parent s..." | Re-trigger Greptile |
There was a problem hiding this comment.
🟢 Approval recommended
The change is limited to test fixtures and makes the previously flaky behavior deterministic without altering production code paths.
Pull request overview
Stabilizes a flaky Delta Lake snapshot pinning test by controlling the filesystem mtime clock used by Delta time travel resolution, without changing production behavior.
Changes:
- Adds a small test helper to stamp
_delta_log/*.jsoncommit files’ mtimes to deterministic values. - Updates
test_resolve_pins_to_last_completed_snapshot_while_parent_is_syncingto pin between two commits spaced minutes apart (instead of relying on millisecond-level mtime resolution). - Removes the prior approach of rewriting
commitInfo.timestamp, which Delta does not use for version resolution.
File summaries
| File | Description |
|---|---|
| products/warehouse_sources/backend/temporal/data_imports/sources/common/rest_source/tests/test_warehouse_parent.py | Makes the parent snapshot pinning test deterministic by setting commit log mtimes and pinning between commits. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…mit gap Adopts the naming and timing from #93192: the helper says it sets commit log mtimes and takes the whole timeline in one call, and the two commits sit 2 minutes apart with the pin 1 minute after the first. Generated-By: PostHog Desktop Task-Id: 4c55bc40-1d25-42fe-9036-908e21493bd1
|
Closing in favor of #93155, which fixes the same flake the same way and has now adopted this PR's helper name and commit-time offsets. Thanks for the probe table — it's what settled that delta-rs ignores |
Problem
Anyone opening a PR that runs the warehouse-sources shard hits a red check that has nothing to do with their change.
test_resolve_pins_to_last_completed_snapshot_while_parent_is_syncinghas failed 103 times across 72 branches since 2026-09-01, 7 of them on master. It also reds the hourly Backend CI schedule on master most hours.Delta time travel picks a version by each commit log's file modification time, not by the timestamp recorded inside the log. The fixture pinned by the recorded timestamp, so the two clocks never had to agree.
The fixture writes two commits back to back. They land 1 to 3 milliseconds apart. Any coarsening of the mtime clock collapses that gap, and the pin then resolves to the in-flight overwrite instead of the completed snapshot.
Changes
_snapshot_pin_as_ofreturns the parent's last completedfinished_at, which falls between the two commits rather than on the first one._set_delta_commit_log_mtimesreplaces the rewrite ofcommitInfo.timestampadded in fix(posthog-ai): convert remaining schema references to generated column tables #84280. Delta ignores that field when it resolves a version, so the rewrite could not close the race.How did you test this code?
The assertion is unchanged, so this guards the same regression as before: a child fan-out must read the parent's last completed snapshot, never a torn overwrite from an in-flight full refresh.
Reproducing the failure needs a coarsened mtime clock, which no test can impose on its own filesystem. A local probe did it directly against
deltalake, flooring both log mtimes before resolving.Probe results, 20 to 30 iterations per row
commitInforewriteThe new fixture also passes with mtimes floored to 60 seconds.
test_warehouse_parent.pypasses locally. The changed test passed 30 consecutive runs.APIBaseTestclasses in the same file. The local test database is in a broken state (relation "django_content_type" already exists), unrelated to this change. CI runs them.Automatic notifications
Docs update
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Written by Claude Code (Opus 5) after a request to explain why Backend CI was red on master.
Skills invoked:
/debugging-ci-failures,/writing-tests,/writing-pr-descriptions.The session found the flake through
hogli ci:insights, then established by probe that delta-rs resolves a time-travel pin against log file mtimes rather thancommitInfo.timestamp. That ruled out the fix already on master and pointed at stamping the mtimes instead.Public artifact: nothing here comes from outside this repository. The probe scripts ran against synthetic Delta tables and are not committed.